fix: raise correct deprecation when config key is top-level in generic test (#12572)#12618
Conversation
…vel of generic test Fixes dbt-labs#12572
ash2shukla
left a comment
There was a problem hiding this comment.
Thanks for this work !
Please address following comments and we can merge it !
8139492 to
0ab062b
Compare
|
Thanks for the review @ash2shukla! Made all three changes — simplified the config key detection, removed the unnecessary profile target fixture, and updated the assertion to == 1. Please take another look! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12618 +/- ##
==========================================
- Coverage 91.38% 91.36% -0.02%
==========================================
Files 203 203
Lines 25627 25707 +80
==========================================
+ Hits 23419 23487 +68
- Misses 2208 2220 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
@kalluripradeep Thanks for making the changes ! Looks like there are some formatting issues. Please make sure you run pre-commit before committing the changes. :) |
this should work now |
Problem
Fixes #12572
Parent: #12394
When a user defines a config property like
whereat the top level of a generic test:dbt was raising
MissingArgumentsPropertyInGenericTestDeprecation, which misleads the developer into movingwhereunderarguments:— which is wrong. The correct fix is to move it underconfig:.Root Cause
In
TestBuilder.extract_test_args(), the check for missingargumentsproperty did not distinguish between actual test arguments and known config properties (CONFIG_ARGSlikewhere,severity,tags, etc.). Any top-level key that wasn'tconfig,column_name,description, ornametriggered the wrong deprecation.Fix
Exclude
CONFIG_ARGSfrom the missing-arguments check inextract_test_args()so that known config properties don't triggerMissingArgumentsPropertyInGenericTestDeprecationEmit
PropertyMovedToConfigDeprecationfor eachCONFIG_ARGSkey found at the root level of the test definition, guiding the developer to move it underconfig:Test
Added
TestMissingArgsVsPropertyMovedToConfigintests/functional/deprecations/test_deprecations.pywhich verifies:PropertyMovedToConfigDeprecationfires forwhereat top levelMissingArgumentsPropertyInGenericTestDeprecationdoes NOT firecc @dbeatty10